-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
4723 org link preview #212
Conversation
This is working correctly as is. Perhaps the cloudfront cache hadn't updated when looking before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I've got a couple non-blocking questions:
- Is accessing properties of
c
object safe without a default? (i.e. c.pkg_dict or c.group_dict) - Is the outputted content what you want? Is it worth writing a test to prove it?
You means it is already deployed to prod? |
No, it is on dev. It could've been something else that caused the delay. I did not investigate. |
I'm not sure. The defaults set in lines 3-7 are to provide defaults in the case that a page is not a resource (e.g., has a
Yes, it does appear correct. There is no good reason to not write a test, I suppose. It would be a best practice, certainly. I didn't partially out of speed and partially that if there is an issue with the |
Thanks for the review, @btylerburton and @FuhuXia! |
Test Org in development showing the right title. |
Related to GSA/data.gov#4723